fix(qwen-code): OAuth headers, WAF detection, and upstream alignment#154
fix(qwen-code): OAuth headers, WAF detection, and upstream alignment#154b3nw wants to merge 1 commit intoMirrowel:devfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughConsolidates Qwen/DashScope authentication defaults, preserves rotated tokens for env-backed credentials, adds WAF-detection and retry for token refreshes, normalizes OAuth API base URLs, updates Qwen endpoints and request headers, and attaches request IDs to device-flow calls. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QwenProvider as Provider
participant TokenEndpoint as OAuthServer
participant DiskEnv as Storage
Client->>Provider: Request action (chat/completion)
Provider->>DiskEnv: Determine credential (file / env:// / api key)
alt credential is env:// or file (oauth)
Provider->>TokenEndpoint: Use access token (may refresh)
TokenEndpoint-->>Provider: 200 JSON or non-JSON (WAF)
alt non-JSON (WAF)
Provider->>TokenEndpoint: Retry with backoff (x attempts)
TokenEndpoint-->>Provider: Final response
end
Provider->>TokenEndpoint: Poll device flow (includes x-request-id)
TokenEndpoint-->>Provider: Token
else direct API key
Provider->>QwenProvider: Call API using DEFAULT_DASHSCOPE_BASE_URL
end
Provider->>Client: Return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| src/rotator_library/providers/qwen_auth_base.py | Adds WAF detection (content-type heuristic), env:// token rotation fix, URL normalization, and constants consolidation; brittle portal.qwen.ai exact-string redirect and a missing .get() guard on expires_in are minor concerns. |
| src/rotator_library/providers/qwen_code_provider.py | Replaces Gemini spoofing headers with DashScope-native headers, adds is_oauth guard for X-DashScope-AuthType, and cleans up constants; uuid and datetime imports are unused. |
Sequence Diagram
sequenceDiagram
participant P as QwenCodeProvider
participant A as QwenAuthBase
participant Q as Qwen Token Endpoint
participant D as DashScope API
P->>A: get_api_details(credential)
A->>A: is_oauth? (env:// or file path)
A->>A: _load_credentials(path)
alt token expired
A->>A: _refresh_token(path)
loop up to 3 retries
A->>Q: POST /oauth2/token (refresh_token)
Q-->>A: HTTP 200
alt content-type contains json
A->>A: parse token, update cache + disk
else WAF block (HTML body)
A->>A: set last_error, exponential backoff sleep
end
end
alt new_token_data is None
A->>A: increment _refresh_failures, set _next_refresh_after
A--xP: raise last_error (backoff applied)
end
end
A->>A: portal.qwen.ai hostname check, use DashScope default
A->>A: _normalize_api_base_url(resource_url)
A-->>P: (api_base, access_token)
P->>D: POST /v1/chat/completions (X-DashScope-AuthType if OAuth)
D-->>P: SSE stream
P->>P: _convert_chunk_to_openai() per chunk
P-->>P: yield litellm.ModelResponse
Reviews (2): Last reviewed commit: "fix(qwen-code): OAuth headers, WAF detec..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/rotator_library/providers/qwen_code_provider.py (1)
610-617:⚠️ Potential issue | 🟡 MinorConditional
X-DashScope-AuthTypeheader required for non-OAuth credentials.
get_api_details()supports direct API keys (when credential_identifier is not a file path or env:// path), but line 617 unconditionally sends"X-DashScope-AuthType": "qwen-oauth". This causes direct API key requests to misidentify their auth mechanism, which skews telemetry and may trip upstream auth validation. The header should be set conditionally based on credential type, or omitted entirely for non-OAuth calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rotator_library/providers/qwen_auth_base.py`:
- Around line 588-595: The env-backed credentials still carry the old injected
portal URL so the fallback to DEFAULT_DASHSCOPE_BASE_URL never triggers; update
the branch that reads creds.get("resource_url") in qwen_auth_base.py to treat a
missing or sentinel portal value as "unset" (e.g. if resource_url is None or
equals "https://portal.qwen.ai/v1") and in that case use
DEFAULT_DASHSCOPE_BASE_URL; keep using
self._normalize_api_base_url(resource_url, DEFAULT_DASHSCOPE_BASE_URL) when
resource_url is a real custom URL, and otherwise set base_url =
DEFAULT_DASHSCOPE_BASE_URL and access_token appropriately (affecting the code
that references resource_url, creds, and credential_identifier).
- Around line 615-619: The normalization logic for resource_url incorrectly
checks url.endswith("/v1") before removing trailing slashes, causing duplicates;
update the code around resource_url/url so you strip trailing slashes first
(e.g., call resource_url.rstrip("/") or url = url.rstrip("/")) and then check
endswith("/v1") and only append "/v1" if missing, ensuring variables like
resource_url, url, and the endswith("/v1") condition are adjusted accordingly.
- Around line 399-414: The token-refresh block in _refresh_token() currently
raises a ValueError on the final non-JSON response, bypassing the shared failure
handling that updates _refresh_failures and _next_refresh_after; instead capture
that failure into last_error (e.g., create the ValueError with the same message
and assign it to last_error) and avoid raising immediately so the function falls
through to the existing "new_token_data is None" branch which uses last_error to
update _refresh_failures/_next_refresh_after; keep the same backoff/continue
behavior for attempt < max_retries - 1 and only set last_error (not raise) when
max_retries is exhausted so the common failure path handles it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99205105-a7b7-4415-b4a6-7a42d96f17fc
📒 Files selected for processing (2)
src/rotator_library/providers/qwen_auth_base.pysrc/rotator_library/providers/qwen_code_provider.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Greptile Review
- GitHub Check: review-pr
🧰 Additional context used
🪛 Ruff (0.15.9)
src/rotator_library/providers/qwen_auth_base.py
[warning] 404-406: Logging statement uses f-string
(G004)
[warning] 411-414: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 572-572: Async functions should not use os.path methods, use trio.Path or anyio.path
(ASYNC240)
[warning] 581-581: Logging statement uses f-string
(G004)
b3nw
force-pushed
the
feature/qwen-code-all
branch
from
3786000 to
0e4993e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/rotator_library/providers/qwen_auth_base.py`:
- Around line 870-875: The current headers dict (containing "x-request-id":
str(uuid.uuid4())) is generated once and reused across the device-code request
and subsequent polling; change the code so a fresh x-request-id is created for
every outbound HTTP call by generating the header value at request time rather
than once at module/flow init — update the places that build/send requests (the
headers construction referenced as headers in qwen_auth_base.py around the
device-code request and the polling loop at the later block) to re-create the
headers dict per call (keeping QWEN_USER_AGENT and Content-Type unchanged) so
each request gets a unique x-request-id.
- Around line 572-598: get_api_details() currently only treats strings starting
with "env://" or existing files as OAuth, so passing a non-existent credential
path (e.g. "oauth_creds/whatever.json") falls through to the direct-API-key
branch; change the detection so credential paths that look like file-based
credentials are treated as OAuth even if the file is missing so
_load_credentials() can perform its env-file fallback. Replace the is_oauth
calculation (the block using os.path.isfile) to mark OAuth when
credential_identifier.startswith("env://") OR
credential_identifier.endswith(".json") OR "/" in credential_identifier (i.e.,
appears to be a path), then continue to call
self._load_credentials(credential_identifier), self._is_token_expired(creds) and
self._refresh_token(credential_identifier) as before.
- Around line 354-365: The current refresh logic unconditionally re-reads from
disk for any path not starting with "env://", which breaks cached credentials
that were originally loaded from env vars via _load_credentials(); change the
condition in the refresh block so _read_creds_from_file(path) is only invoked
when the path actually points to an existing file (e.g., check file existence
before calling _read_creds_from_file), otherwise use the cached entry in
self._credentials_cache[path]; update the condition around the call to
_read_creds_from_file in the refresh flow that references path,
_read_creds_from_file, and _credentials_cache so env-backed cached entries
aren’t forced to hit disk.
In `@src/rotator_library/providers/qwen_code_provider.py`:
- Around line 610-624: The duplicate env:// / file-path credential detection in
QwenCodeProvider should be replaced with the shared helper in QwenAuthBase to
keep token-source logic centralized: remove the try/except block that computes
is_oauth from credential_path and instead call the existing
QwenAuthBase.get_api_details (or extract a small is_oauth helper from it) to
determine whether the token is OAuth, then set headers["X-DashScope-AuthType"] =
"qwen-oauth" when that helper indicates OAuth; update QwenCodeProvider to
reference the QwenAuthBase method (e.g., via self.auth_base.get_api_details or
the extracted helper) so the token classification and header logic remain in
lockstep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8416d8b8-043d-4e00-8b21-c1d71348a06d
📒 Files selected for processing (2)
src/rotator_library/providers/qwen_auth_base.pysrc/rotator_library/providers/qwen_code_provider.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
🪛 Ruff (0.15.9)
src/rotator_library/providers/qwen_code_provider.py
[warning] 611-611: Async functions should not use os.path methods, use trio.Path or anyio.path
(ASYNC240)
src/rotator_library/providers/qwen_auth_base.py
[warning] 408-410: Logging statement uses f-string
(G004)
[warning] 573-573: Async functions should not use os.path methods, use trio.Path or anyio.path
(ASYNC240)
[warning] 582-582: Logging statement uses f-string
(G004)
| # [ROTATING TOKEN FIX] Read fresh credentials before refresh. | ||
| # Qwen uses rotating refresh tokens - each refresh invalidates the previous token. | ||
| # If we use a stale cached token, refresh will fail with HTTP 400. | ||
| # Reading fresh from disk ensures we have the latest token. | ||
| await self._read_creds_from_file(path) | ||
| if not path.startswith("env://"): | ||
| # For file paths, read fresh from disk to pick up tokens that may have | ||
| # been updated by another process or a previous refresh cycle. | ||
| await self._read_creds_from_file(path) | ||
| # For env:// paths, the in-memory cache is the single source of truth. | ||
| # _save_credentials updates the cache after each refresh, so the cache | ||
| # always holds the latest rotating tokens. Re-reading from static env vars | ||
| # would discard the rotated refresh_token and break subsequent refreshes. | ||
| creds_from_file = self._credentials_cache[path] |
There was a problem hiding this comment.
Don't force a disk reread for cached env-backed credentials.
_load_credentials() still supports the backwards-compatible path where a missing file falls back to env vars and caches the result (src/rotator_library/providers/qwen_auth_base.py Lines 230-242). This branch only exempts env://, so refreshing one of those cached env-backed credentials now raises on the nonexistent file before the refresh even starts.
Suggested fix
- if not path.startswith("env://"):
+ loaded_from_env = cached_creds and cached_creds.get(
+ "_proxy_metadata", {}
+ ).get("loaded_from_env")
+ if not path.startswith("env://") and not loaded_from_env:
# For file paths, read fresh from disk to pick up tokens that may have
# been updated by another process or a previous refresh cycle.
await self._read_creds_from_file(path)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/qwen_auth_base.py` around lines 354 - 365, The
current refresh logic unconditionally re-reads from disk for any path not
starting with "env://", which breaks cached credentials that were originally
loaded from env vars via _load_credentials(); change the condition in the
refresh block so _read_creds_from_file(path) is only invoked when the path
actually points to an existing file (e.g., check file existence before calling
_read_creds_from_file), otherwise use the cached entry in
self._credentials_cache[path]; update the condition around the call to
_read_creds_from_file in the refresh flow that references path,
_read_creds_from_file, and _credentials_cache so env-backed cached entries
aren’t forced to hit disk.
| try: | ||
| is_oauth = credential_identifier.startswith("env://") or os.path.isfile( | ||
| credential_identifier | ||
| ) | ||
| except (OSError, ValueError): | ||
| # os.path.isfile can raise on invalid path strings (e.g. very long API keys) | ||
| is_oauth = False | ||
|
|
||
| if is_oauth: | ||
| lib_logger.debug( | ||
| f"Using OAuth credentials from file: {credential_identifier}" | ||
| f"Using OAuth credentials from: {credential_identifier}" | ||
| ) | ||
| creds = await self._load_credentials(credential_identifier) | ||
|
|
||
| if self._is_token_expired(creds): | ||
| creds = await self._refresh_token(credential_identifier) | ||
|
|
||
| base_url = creds.get("resource_url", "https://portal.qwen.ai/v1") | ||
| if not base_url.startswith("http"): | ||
| base_url = f"https://{base_url}" | ||
| resource_url = creds.get("resource_url") | ||
| if resource_url == "https://portal.qwen.ai/v1": | ||
| resource_url = None | ||
| base_url = self._normalize_api_base_url(resource_url, DEFAULT_DASHSCOPE_BASE_URL) | ||
| access_token = creds["access_token"] | ||
| else: | ||
| # Direct API key: use as-is | ||
| # Direct API key: use as-is with DashScope default | ||
| lib_logger.debug("Using direct API key for Qwen Code") | ||
| base_url = "https://portal.qwen.ai/v1" | ||
| base_url = DEFAULT_DASHSCOPE_BASE_URL | ||
| access_token = credential_identifier |
There was a problem hiding this comment.
This classifier bypasses the documented missing-file env fallback.
Only env:// and existing files are treated as OAuth here. That makes the _load_credentials() fallback for nonexistent file paths (src/rotator_library/providers/qwen_auth_base.py Lines 230-242) unreachable from get_api_details(), so a stateless deployment that still passes oauth_creds/...json will now send the literal path as the bearer token.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 573-573: Async functions should not use os.path methods, use trio.Path or anyio.path
(ASYNC240)
[warning] 582-582: Logging statement uses f-string
(G004)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/qwen_auth_base.py` around lines 572 - 598,
get_api_details() currently only treats strings starting with "env://" or
existing files as OAuth, so passing a non-existent credential path (e.g.
"oauth_creds/whatever.json") falls through to the direct-API-key branch; change
the detection so credential paths that look like file-based credentials are
treated as OAuth even if the file is missing so _load_credentials() can perform
its env-file fallback. Replace the is_oauth calculation (the block using
os.path.isfile) to mark OAuth when credential_identifier.startswith("env://") OR
credential_identifier.endswith(".json") OR "/" in credential_identifier (i.e.,
appears to be a path), then continue to call
self._load_credentials(credential_identifier), self._is_token_expired(creds) and
self._refresh_token(credential_identifier) as before.
| headers = { | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36", | ||
| "User-Agent": QWEN_USER_AGENT, | ||
| "Content-Type": "application/x-www-form-urlencoded", | ||
| "Accept": "application/json", | ||
| "x-request-id": str(uuid.uuid4()), | ||
| } |
There was a problem hiding this comment.
Regenerate x-request-id per outbound request.
x-request-id is created once and then reused for the device-code request plus every polling call, so the telemetry is per-flow, not per-request.
Suggested fix
- headers = {
+ base_headers = {
"User-Agent": QWEN_USER_AGENT,
"Content-Type": "application/x-www-form-urlencoded",
"Accept": "application/json",
- "x-request-id": str(uuid.uuid4()),
}
async with httpx.AsyncClient() as client:
request_data = {
"client_id": CLIENT_ID,
"scope": SCOPE,
@@
dev_response = await client.post(
"https://chat.qwen.ai/api/v1/oauth2/device/code",
- headers=headers,
+ headers={**base_headers, "x-request-id": str(uuid.uuid4())},
data=request_data,
)
@@
poll_response = await client.post(
TOKEN_ENDPOINT,
- headers=headers,
+ headers={**base_headers, "x-request-id": str(uuid.uuid4())},
data={
"grant_type": "urn:ietf:params:oauth:grant-type:device_code",
"device_code": dev_data["device_code"],
"client_id": CLIENT_ID,
"code_verifier": code_verifier,Also applies to: 947-956
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/qwen_auth_base.py` around lines 870 - 875, The
current headers dict (containing "x-request-id": str(uuid.uuid4())) is generated
once and reused across the device-code request and subsequent polling; change
the code so a fresh x-request-id is created for every outbound HTTP call by
generating the header value at request time rather than once at module/flow init
— update the places that build/send requests (the headers construction
referenced as headers in qwen_auth_base.py around the device-code request and
the polling loop at the later block) to re-create the headers dict per call
(keeping QWEN_USER_AGENT and Content-Type unchanged) so each request gets a
unique x-request-id.
| try: | ||
| is_oauth = credential_path.startswith("env://") or os.path.isfile(credential_path) | ||
| except (OSError, ValueError): | ||
| is_oauth = False | ||
|
|
||
| headers = { | ||
| "Authorization": f"Bearer {access_token}", | ||
| "Content-Type": "application/json", | ||
| "Accept": "text/event-stream", | ||
| "User-Agent": "google-api-nodejs-client/9.15.1", | ||
| "X-Goog-Api-Client": "gl-node/22.17.0", | ||
| "Client-Metadata": "ideType=IDE_UNSPECIFIED,platform=PLATFORM_UNSPECIFIED,pluginType=GEMINI", | ||
| "User-Agent": QWEN_USER_AGENT, | ||
| "X-DashScope-CacheControl": "enable", | ||
| "X-DashScope-UserAgent": QWEN_USER_AGENT, | ||
| } | ||
| if is_oauth: | ||
| headers["X-DashScope-AuthType"] = "qwen-oauth" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract OAuth credential detection into one shared helper.
This block reimplements the same env:///file-path heuristic as QwenAuthBase.get_api_details() in src/rotator_library/providers/qwen_auth_base.py Lines 572-579. The token source and X-DashScope-AuthType need to stay in lockstep, so keeping two copies will drift again the next time credential classification changes.
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 611-611: Async functions should not use os.path methods, use trio.Path or anyio.path
(ASYNC240)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/rotator_library/providers/qwen_code_provider.py` around lines 610 - 624,
The duplicate env:// / file-path credential detection in QwenCodeProvider should
be replaced with the shared helper in QwenAuthBase to keep token-source logic
centralized: remove the try/except block that computes is_oauth from
credential_path and instead call the existing QwenAuthBase.get_api_details (or
extract a small is_oauth helper from it) to determine whether the token is
OAuth, then set headers["X-DashScope-AuthType"] = "qwen-oauth" when that helper
indicates OAuth; update QwenCodeProvider to reference the QwenAuthBase method
(e.g., via self.auth_base.get_api_details or the extracted helper) so the token
classification and header logic remain in lockstep.
|
There are some notable things raised in the comments. Gonna apply some fixes and then merge |
2 participants
Summary
This PR refactors the Qwen Code provider to improve stability in token rotation, clean up request headers, and add robust mitigations against upstream Alibaba Cloud WAF blocks. It also resolves a structural circular dependency in the provider imports.
Key Changes
DEFAULT_DASHSCOPE_BASE_URLandQWEN_USER_AGENTconstants to the lower-levelqwen_auth_base.pymodule to establish a single source of truth without fragile delayed imports.200 OK(HTML payload) interceptions and trigger an exponential backoff rather than crashing the refresh cycle with aJSONDecodeError.env://Token Rotation: Updatedproactively_refresh()to bypass fresh disk reads for virtualenv://targets. This preserves in-memory rotated tokens and resolves theHTTP 400 Bad Requestloop during subsequent refreshes./v1suffixes, fixing the previous duplication bug (/v1/v1/chat/completions).X-DashScope-*headers and a unified User-Agent, supplemented by request UUIDs for improved telemetry tracking.